Code Review: Region-aware CLI profile support

Reviewed the full diff, checked out the branch, and ran build/format/tests locally:

• go test ./internal/cli/ → ok
• gofmt -l → clean · go vet → clean

This is a well-structured, well-documented change. Region logic is cleanly centralized in the new region.go, the breaking changes are honestly flagged in the CHANGELOG/PR body, and test coverage for the new code paths is solid. Approve with minor changes.

Issues

1. Dead condition + misleading comment in the region-mismatch guard (internal/cli/projects.go:139-143)

sessionRegion := currentRegionFromContext(ctx)
if bindingRegion != "" && sessionRegion != "" && !strings.EqualFold(bindingRegion, sessionRegion) {

The comment says "An empty session region (fresh login default) is treated as 'no opinion' and does not conflict." But currentRegionFromContext normalizes empty → "global", so sessionRegion is never "". The sessionRegion != "" clause is dead, and the documented "no opinion" escape hatch is unreachable.

Practical consequence: a repo bound to cn while the context is the default global (e.g. user never logged in, or logged into global) hard-fails with PROJECT_REGION_MISMATCH. For the logged-into-global case that's intended; for the never-logged-in case it pre-empts the auth error with a region error. Arguably acceptable guidance, but the comment should be corrected to match reality (or, if "no opinion" was genuinely intended for unauthenticated state, detect session presence rather than the always-normalized region).

2. Unrelated changes bundled in — scope creep (internal/cli/quickstart.go)
isLocalGitRepoURL, the GIT_ALLOW_PROTOCOL=file env, and skipping --depth 1 for local paths have nothing to do with region profiles — they look like test-fixture support for cloning local repos. Mixing this into a region PR makes the diff harder to reason about, and the file:// protocol enablement is a (small) security-surface change that deserves its own review. Consider splitting it out, or at least calling it out explicitly in the PR description.

Minor / Nits

• CN quickstart URLs are no-ops. Every RepoURLCN/DocsURLCN is identical to its global counterpart. The quickstartRepoURLForRegion/quickstartDocsURL machinery + tests currently change nothing behaviorally — it's forward-looking scaffolding. Fine to land, but worth a comment noting the CN values are placeholders.
• open_targets.go duplication. The cn and global switch blocks are identical for docs/docs-md and differ only for console/product-docs. Two near-duplicate switches invite drift; a small per-region URL map keyed by target would be tighter.
• authStatus region source. region now comes from a.authRegion() (context) rather than the session. If the context file is cleared while a session persists, status would report global regardless of the control plane that issued the token. Edge case, low impact.
• Shared a.projectEnvProject field backs two flag registrations (commands.go:783, :871). Safe today since one command runs per invocation, but it's mutable shared state on App.

Done well

• Endpoint override precedence correctly uses osEnv (the pre-applyConfigToEnv snapshot), so only genuine user overrides bypass region defaults — and this is directly tested.
• Migration correctly drops the four legacy keys on load, with file-level assertions in app_test.go.
• New PROJECT_REGION_MISMATCH is registered in docs/error-codes.md, and the error message is actionable.
• Region resolution is consolidated into one small, well-tested file instead of scattered if region == "" { region = "global" } blocks — a genuine simplification.

🤖 Generated with Claude Code

Correction: reset-to-global on agora login is intended

Author confirms the bare-agora login → global behavior is by design, not a regression. Withdrawing the request changes escalation from my follow-up review — back to approve with minor changes.

Two things still worth addressing, though:

1. The interactive login path is now inconsistent with the explicit one. Explicit agora login (no flag) resets to global, but login-on-demand preserves the current region: promptForLogin → loginPromptRegion() → authRegionFromContext() (auth.go:564). So a cn user who types agora login lands on global, while a cn user who lets a command prompt them stays on cn. If reset-to-global is the intended semantic for explicit login, decide whether the interactive path should reset too — right now the two disagree. (If preserving is correct for interactive, that's defensible, but it should be a conscious choice and ideally commented.)

2. Document the reset in the CHANGELOG. It's a user-visible change from the old preferred-region behavior (bare login used to keep your last region). A one-line note under Changed, and optionally a "switching from cn to global" stderr notice, would prevent surprise.

Everything else from the consolidated checklist still stands at its original priority (items 3–7).

🤖 Generated with Claude Code